@claude review

Claude finished @cliffhall's task in 8m 9s —— View job

Review: gate src/lib/** under the coverage include globs

• Read the PR diff
• Review vite.config.ts include glob change
• Review downloadFile.ts v8-ignore annotation
• Review new test files
• Check for untested siblings in src/lib now under the gate
• Ran npm run test:coverage locally to verify the gate

Verdict: LGTM. This is a clean, low-risk config + test PR that does exactly what it says. I ran the full web coverage gate locally — 3366 tests pass, exit 0, and every src/lib module reports 100/100/100/100.

What's good

• The glob change is right. src/lib/**/*.{ts,tsx} now matches the form of the existing src/components/** / src/utils/** entries, retiring the single-file sandbox-csp workaround from feat(web): add sandbox CSP builder library (closes #1558) #1588. No new file slips through untested: the four gated modules (downloadFile, environmentFactory, remoteOAuthStorage, types/customHeaders) plus sandbox-csp all clear the gate, and .test.ts / index.ts / .d.ts are excluded by the existing coverage.exclude patterns, so the glob doesn't pull in test files.
• The v8 ignore is correctly justified. String.prototype.split always returns a non-empty array (min [""]), so .pop() is never undefined and ?? "" is provably dead — exactly the "provably-dead defensive guard" case AGENTS.md sanctions for a source annotation rather than lowering the gate. Scoping it to /* v8 ignore next */ on that one line is the minimal blast radius.
• Tests are thorough. customHeaders.test.ts exercises all three branches of createHeaderFromBearerToken (default name, authorization case-insensitive, non-auth raw token) plus the trim/filter logic. environmentFactory.test.ts mocks the three remote factories, asserts baseUrl/authToken are threaded into each, and — nicely — captures and invokes the internal fetchFn wrapper to prove it delegates to globalThis.fetch (the one arrow body that would otherwise be an uncovered function).

Minor (non-blocking) nits

• environmentFactory.test.ts:76 — the createRemoteFetch mock returns an async () => new Response("ok") that's never invoked; a bare vi.fn() would read more clearly as "unused stub." Cosmetic.
• The "passes an undefined auth token straight through" test asserts only captured.logger?.authToken. Since all three factories receive the same authToken, this is fine, but asserting on transport/fetch too would make the intent (all three get undefined) explicit. Optional.

Neither is worth a change. Ship it.
· 1594-gate-src-lib

📋 Smoke test & AGENTS.md audit for this PR: #1594 (comment) (posted on the linked issue #1594).